fix(config): mask the Euro-Office jwt_secret as sensitive - #63311
Conversation
Both sensitive-key allowlists carried an `onlyoffice` entry but nothing for `eurooffice`, so the document server signing key was printed in full while the ONLYOFFICE one next to it was redacted. SystemConfig covers `occ config:list system`, which is where the key lands when the app is configured through config.php. AppConfig covers plain `occ config:list` and the admin support report, which is where it lands when it is set through the app's own settings page. Fixes: #63302 Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Christoph Schaefer <christoph.schaefer@nextcloud.com>
|
Duplicate of #63306 |
moodyjmz
left a comment
moodyjmz
left a comment
There was a problem hiding this comment.
Verified: same fix as #63306 in both SystemConfig::DEFAULT_SENSITIVE_VALUES and AppConfig::getSensitiveKeys(), but this one adds regression tests for both paths (including the first unit test for SystemConfigTest, which had none), keeps the sensitive-key array alphabetically ordered, and the PR body correctly documents why both files need the change — the secret leaks via two independent paths depending on whether the admin configures it through config.php or the app's own settings page. Companion fix on the app side (Euro-Office/eurooffice-nextcloud#134) sets VALUE_SENSITIVE at write time, which is the durable fix; this covers instances/app-versions that predate it.
CarlSchwan
added
the
4. to release
|
Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22 |
|
/backport to stable34 |
5 participants
Fixes bug #63302 "Eurooffice jwt_secret is exposed in system report"
lib/private/SystemConfig.php'sDEFAULT_SENSITIVE_VALUESandlib/private/AppConfig.php'sgetSensitiveKeys()both carry anonlyofficeentry and neither has one for
eurooffice, so the Euro-Office document serversigning key is printed in full — in the reporter's case directly next to a
redacted ONLYOFFICE one in the same output. This adds the missing entry to both
lists.
Both are needed because the key reaches two different places depending on how
the admin configures the app. Set through
config.php, it lands in systemconfig and leaks via
occ config:list system, which is what #63302 reports.Set through the app's own admin settings page, it lands in app config and leaks
via plain
occ config:listand the Support → system report — the pathnextcloud/all-in-one#8317 originally described before being closed and pointed
here.
The connector app is also setting
VALUE_SENSITIVEon the key itself, inEuro-Office/eurooffice-nextcloud#, so it no longer depends on this hardcoded
list at all. The
AppConfighalf here is still worth having: the flag onlytakes effect when the secret is next written, so keys already stored by earlier
app versions stay exposed until then.
tests/lib/SystemConfigTest.phpis new — the class had no unit test at all —and covers
getFilteredValue()maskingjwt_secretwhile leaving itsnon-sensitive siblings intact.
tests/lib/AppConfigTest.phpgains theequivalent case for
getFilteredValues(). Both seedlazyCachesoloadConfig()returns from cache, and assertgetQueryBuilder()is nevercalled, keeping them database-free.
Severity is low — it takes an admin voluntarily publishing a report — which is
why this is a normal PR rather than a HackerOne report. Same handling as
6c0b862, which added the ONLYOFFICE entry in public. Worth noting for anyone
who already posted a
config:listdump to the forum: that dump contains a livesigning key and it should be rotated.
Euro-Office ships for server 33 and up, so this probably wants a backport to
stable33.
Assisted-by: ClaudeCode:claude-opus-5